-
Notifications
You must be signed in to change notification settings - Fork 42
✨ Neutral Atom QDMI Device #996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@burgholzer This PR is far from being ready, that's why it is labeled as For context: The protobuf file in |
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huge props @ystade. This was extremely pleasant to browse through. The current setup feels really solid and I believe all of my comments (which are still quite some), are on fine details and tiny little recommendations that you might have planned anyway. I hope they still provide you with additional food for thought to push this further.
One interesting aspect that I have not yet thought about is how this will play out on Windows once a Driver is involved. But that's something to tackle in a separate PR.
The new dependency of When doing that, Now, the ugly part comes: The cmake configuration of However, when we want to ship the QDMI NA device at some point with the Python wheel, we need to be very careful what we include in the Python wheel. Because of #1021 together with the fact that the install targets of |
In general, concerning |
## Description This PR does not immediately solve an issue but contributes to avoiding an issue that arose in #996. In particular, it allows to select the install components explicitly and, hence, avoid installing components that were added by dependencies where install instructions cannot be deactivated, e.g., RE2. Also in general, I think, it is a good idea to set the install components for the wheel explicitly. ## Checklist: <!--- This checklist serves as a reminder of a couple of things that ensure your pull request will be merged swiftly. --> - [x] The pull request only contains commits that are focused and relevant to this change. - [ ] I have added appropriate tests that cover the new/changed functionality. - [ ] I have updated the documentation to reflect these changes. - [ ] I have added entries to the changelog for any noteworthy additions, changes, fixes or removals. - [ ] I have added migration instructions to the upgrade guide (if needed). - [x] The changes follow the project's style guidelines and introduce no new warnings. - [x] The changes are fully tested and pass the CI checks. - [x] I have reviewed my own code changes. --------- Signed-off-by: Yannick Stade <[email protected]> Co-authored-by: Lukas Burgholzer <[email protected]>
* @param argc is the number of command line arguments. | ||
* @param argv is the array of command line arguments. | ||
*/ | ||
int main(int argc, char* argv[]) { |
Check warning
Code scanning / CodeQL
Poorly documented large function Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
To fix the issue, we will add inline comments to the main
function to document its logic and flow. These comments will explain:
- The purpose of each major block of code (e.g., parsing arguments, handling errors, processing commands).
- The rationale behind specific operations (e.g., why certain exceptions are caught or why certain conditions are checked).
The changes will be made directly within the main
function in src/na/device/App.cpp
. No new methods, imports, or definitions are required.
-
Copy modified line R280 -
Copy modified line R286 -
Copy modified line R291 -
Copy modified line R294 -
Copy modified line R299 -
Copy modified line R302
@@ -279,2 +279,3 @@ | ||
int main(int argc, char* argv[]) { | ||
// Convert command-line arguments into a vector of strings for easier processing. | ||
std::vector<std::string> argVec; | ||
@@ -284,2 +285,3 @@ | ||
} catch (std::exception& e) { | ||
// Log an error if argument parsing fails and exit with a non-zero status. | ||
SPDLOG_ERROR("Error parsing arguments into vector: {}", e.what()); | ||
@@ -288,4 +290,6 @@ | ||
try { | ||
// Parse the arguments into a structured format for further processing. | ||
parsedArgs = parseArguments(argVec); | ||
} catch (const std::exception& e) { | ||
// Log an error if argument parsing fails and display usage information. | ||
SPDLOG_ERROR("Error parsing arguments: {}", e.what()); | ||
@@ -294,4 +298,6 @@ | ||
} | ||
// Extract parsed arguments and the index of the first sub-command argument. | ||
const auto& [args, i] = parsedArgs; | ||
if (args.help) { | ||
// If the help flag is set, display usage information and exit successfully. | ||
printUsage(args.programName); |
Cpp-Linter Report
|
Description
This PR adds a universal QDMI device implementation for neutral atom-based quantum computers. The device itself is specified in a JSON file whose structure is given by a protobuf definition. During compilation time, the JSON file is read and translated to C++ code similar to how TableGen works in the LLVM project.
Subsequent PRs will add a QDMI driver utilizing the new device and a FoMaC tailored to those neutral atom devices.
Checklist: